…ecution on (status, created_at) (#2264)

* UN-3972 [PERF] Index workflow_file_execution on (status, created_at)

The dashboard metrics cron's documents_processed and failed_pages queries
filter this table on status + a created_at window, but all four existing
indexes lead with workflow_execution_id. With no entry point here the planner
drives top-down from the org and sequentially scans all 1.28M rows of
workflow_execution — 83% of the cron's DB time on production.

Built CONCURRENTLY with atomic = False; a plain AddIndex would hold a SHARE
lock over a 3.4GB table taking live inserts. Guarded against a leftover
INVALID index from an interrupted build, which IF NOT EXISTS would otherwise
keep silently.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc

* UN-3972 [PERF] Trim the migration docstring to the project ceiling

The docstring restated the prod plan, deployment runbook and recovery steps.
That detail belongs in the PR, not in a file every future agent scans.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc

* UN-3972 [PERF] Guard the index migration's non-atomic CONCURRENTLY shape with tests

The suite runs with --no-migrations, so 0007 is never executed in CI. Regenerating it
with makemigrations, or dropping atomic = False / CONCURRENTLY while tidying, would land
a plain AddIndex — a SHARE lock held for the whole build on a 3.4 GB table that takes
live inserts — with every test still green.

Five DB-free assertions on the migration module and the model's Meta.indexes: non-atomic,
concurrent in both directions, the INVALID-index guard present, AddIndex confined to
state_operations, and model/migration agreement.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc

* UN-3972 [FIX] Assert the index definition, not just its validity, and pin reversibility

The CREATE INDEX CONCURRENTLY IF NOT EXISTS matches on name alone, so a hand-built
index with different columns was kept while Django recorded (status, created_at) into
model state — a permanent, invisible divergence that makemigrations --check cannot
see. The guard now compares pg_get_indexdef against the expected btree definition and
qualifies the lookup by current_schema(), since app tables live in the unstract schema.

Also pin that every database_operation is reversible: dropping the guard's
reverse_sql=noop killed the whole rollback path with all five tests still green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc

* UN-3972 [FIX] Address Athul's review: guard semantics, whole-migration assertions

Three mutations that were green are now caught: appending a bare AddIndex after the
SeparateDatabaseAndState (a real lock-taking build on a 3.4 GB table, invisible because
every assertion read operations[0]); flipping the guard's NOT indisvalid polarity, which
either raises on every healthy deploy or never fires at all; and a typo in reverse_sql,
which makes rollback a silent no-op through IF EXISTS while Django unapplies the
migration.

The CREATE assertion matches the column order by regex instead of an exact byte
sequence — removing one space used to fail it, a false-failure mode whose only outcome
is someone loosening the assertion.

Docstrings: the plan citation now points at UN-4045, which supersedes the earlier
workflow_file_execution reading; "every existing index leads with workflow_execution_id"
was false (the PK leads with id); the exact CREATE statement an operator should run out
of band is spelled out, with a warning off the struck two-index variant; and the models.py
comment no longer implies the index fixes both cron queries when it fixes one until
UN-3973 narrows the window.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

…edule by tier and indexing workflow_execution on created_at (#2265)

* UN-3973 Derive monthly metrics from the daily tier, narrow source window to 2 days

The dashboard aggregation widened its DAY-granularity query to the first of
the previous month so monthly buckets could be summed in Python from the same
rows. Every run re-read 32-62 days of source data per metric, per org, 96
times a day.

Monthly is now rolled up from event_metrics_daily in one statement for all
orgs, so the source queries only need the daily window. That window drops to
2 days, sized against the measured worst created_at -> terminal-status lag of
~2h. A once-daily 7-day pass reruns the same task at a wider bound to repair
gaps left by cron downtime.

The active-org prefilter is decoupled from the daily window and pinned at 7
days: metrics filtered on another column (hitl_completions on approved_at) can
land for an org whose executions are older than the source window.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc

* UN-3973 Address Sonar and Greptile review findings

Sonar:
- S117: rename apps.get_model() locals in 0004 to snake_case
- S3776: cut _run_aggregation cognitive complexity from 22 by hoisting the
  static metric config tables to module level and extracting the per-org
  body, the active-org prefilter and the result shape into helpers

Greptile:
- Monthly rows in the rebuilt window whose daily rows are gone are now
  deleted alongside the upsert, so the two tiers cannot disagree. An empty
  daily tier still short-circuits, so a wiped tier cannot cascade into
  deleting monthly history.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc

* UN-3973 Add tests covering the source window, rollup SQL and reconciliation schedule

Closes the acceptance criteria that had no automated check:

- the monthly rollup issues no source-table SQL, asserted by capturing the
  queries it actually sends
- the window ladder at 2 / 7 / 62 days, including a row that finishes after
  the narrow window has moved past its created_at and so never re-enters it
- the reconciliation schedule row, its idempotency and its reverse

The schedule tests call the migration's function directly. The suite runs with
--no-migrations, so data migrations never execute and asserting on the beat row
would fail regardless of the migration being correct.

Also moves the dotenv load in settings/base.py above the Celery block.
CELERY_BROKER_BASE_URL, _USER and _PASS were read above it, so they could not be
supplied by an env file at all and had to be ambient. Ambient values still take
precedence, so deployed behaviour is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc

* UN-3973 Trim comments in tasks.py and revert the unrelated settings change

Cut the verbose comments and docstrings down to the purpose and the
non-obvious bits. Code is unchanged.

Restore backend/settings/base.py to main — moving the dotenv load ahead of
get_required_setting was a local test convenience, not part of this change.
The test rig exports the broker vars itself, so CI never needed it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc

* UN-3974 [PERF] Split the dashboard metrics schedule by tier and index workflow_execution on created_at

Schedule split. One schedule ran every 15 minutes and wrote all three metric
tiers. Dashboard daily and monthly figures do not need 15-minute freshness, so
they move to hourly — 96 runs a day becomes 24 for the expensive
DAY-granularity half of the work, while the hourly tier keeps its cadence.

Both schedule rows point at the same task and differ only in a `tier` kwarg; a
second task name would need its own worker registration and internal endpoint
for the PG path. The lock is now keyed per tier, so the two runs that collide
at the top of every hour do not starve each other. Omitting `tier` still writes
all three tiers, so a manual trigger never silently writes nothing.

Prefilter index. The active-org prefilter measures 1,849ms per call on
production — the slowest single query on the instance. Nothing on
workflow_execution leads with created_at: the two composite indexes are
date-ordered only within one workflow or pipeline, and the partial index is
empty in steady state. The split raises this query's call count, and UN-4045
will leave three more metric queries on the same bare date-range shape, so the
index lands with the split rather than after it.

Built CONCURRENTLY with atomic = False and guarded against a leftover INVALID
index, matching migration 0026.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc

* UN-3974 [PERF] Keep scheduler ownership out of the split migration and cut _run_aggregation's complexity

Migration 0005 used update_or_create for the PG row of the schedule it was only
re-keying, which reset pg_owned to False. converge_pg_scheduler disables a row's
Beat twin when the PG scheduler adopts it, so on an adopted deployment the
migration would have left the aggregation with no firer at all — Beat disabled,
PG no longer owning it. It now updates only task_kwargs on that row, leaving
enabled and pg_owned to the scheduler that owns them. Rollback is symmetric.

Threading the tier through _run_aggregation took its cognitive complexity from
25 to 27 against a limit of 15. Extracted _collect_org_metrics and
_aggregate_org, and hoisted the two static metric tables to module level so they
are not rebuilt per call. Names match the same extraction on #2255 so the two
reconcile cleanly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc

* UN-3974 [PERF] Trim comments and docstrings to the project ceiling

The two index migrations carried 50-60 line docstrings restating the prod plan,
deployment runbook and recovery steps. That detail belongs in the PR, not in
files every future agent scans. Cut to purpose and key behaviour.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc

* UN-3974 [PERF] Cover all three acceptance criteria with tests

The suite runs with --no-migrations, so neither 0005 nor 0029 ever executes in
CI, and nothing pinned the schedule split's behaviour at all. 46 tests, at least
one per acceptance criterion.

AC-1 — cadence, and the tier reaching the task. 0005 creates one row and rewrites
one, both scheduler tables agreeing, and the rewrite touches neither pg_owned nor
enabled: on an adopted deployment converge_pg_scheduler has already disabled the
Beat twin, so handing ownership back would leave the aggregation with no firer.
Separately the internal endpoint and the worker proxy are pinned to carry `tier`
— that leg fails silently, since _call_internal builds a body only when a tier is
given and the existing worker test called the task without one.

AC-2 — the split changes no figure. Runs the real _run_aggregation three times
and diffs the metrics tables: `hourly` reproduces the pre-split hourly figures
exactly, and hourly + daily_monthly reproduce every row `all` writes. Two guards
keep it from going vacuous, the second because mutation testing caught the first
version passing while _aggregate_single_metric was broken — the fixture produced
only LLM metrics, leaving half the split unverified.

AC-3 — the index. Migration shape (non-atomic, CONCURRENTLY both directions, the
INVALID guard, AddIndex confined to state_operations), plus an integration test
that EXPLAINs the query the aggregation actually issues, captured rather than
rewritten: a hand-copied queryset would keep passing after the prefilter changed,
which is the one thing it is for. Rows are inserted in ascending created_at order
so the heap matches production's append order. The Query Insights half of AC-3 is
a production reading and is deliberately not faked here.

Every test verified to fail when the thing it guards breaks.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc

* UN-3973 Renumber the reconciliation migration to 0005

UN-3445 landed 0004_pg_periodic_tasks on main after this branch was cut, leaving
dashboard_metrics with two 0004s depending on 0003 and nothing depending on either.
Django saw two leaf nodes and refused to build the graph, so `migrate` failed before
applying anything — every app, not just this one.

Depend on 0004_pg_periodic_tasks and renumber to match, so the short prefix form
stays usable for a rollback.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc

* UN-3974 Renumber the schedule-split migration to 0006 behind UN-3973's 0005

UN-3445's 0004_pg_periodic_tasks is the parent of both this migration and UN-3973's
reconciliation migration, so landing both would leave dashboard_metrics with two leaf
nodes and no applicable graph.

Depend on 0005_add_reconciliation_task instead, which puts the intended merge order
(UN-3973 then UN-3974) in the graph rather than in the merge queue. This branch cannot
migrate on its own until UN-3973 lands; its tests are unaffected, since the suite runs
with --no-migrations.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc

* UN-3973 [FIX] Address review: PG transport, scoped orphan sweep, retry posture

The reconciliation row could not run on the PG transport — two functions share the
task name dashboard_metrics.aggregate_from_sources and the worker one took no
arguments, so the mirrored row dispatched source_window_days into a zero-arg function
and the message was dropped. The worker proxy and the internal endpoint now plumb it,
and 0005 declares the PG twin rather than leaving the mirror to invent one.

The orphan sweep is scoped to the (organization, month) partitions the rollup actually
produced: an incomplete daily tier passed the empty-tier guard and deleted monthly rows
it could not vouch for. Its deletion count now reaches the task result and a WARNING.

DatabaseError and OperationalError propagate from the monthly rollup so the configured
autoretry fires, instead of being logged once behind success: True.

The prefilter is never narrower than the query window, so a widened source_window_days
cannot skip the orgs it exists to repair. bulk_create takes an explicit batch_size.

The Beat/PG drift guard named 0002 and 0004, so it kept comparing three schedules
against three while this PR added a fourth. It now discovers every migration in the
app, replays their RunPython forwards in order, derives the Beat cadence from the
schedule row, binds every declared kwarg to its task signature, and asserts every
post-install Beat write bumps PeriodicTasks.last_update.

The reconcile row moves to 04:40 UTC: */15 fires at minute 0, and the per-tier lock
keys do not block each other, so 04:00 started two full aggregations at once.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc

* UN-3974 [FIX] Address review: Beat reload, inherited ownership, boundary validation

Rewriting live Beat rows through historical models fires no post_save, so
DatabaseScheduler never reloaded: the existing row kept firing with no tier and the
new row never fired at all. 0006 now bumps PeriodicTasks.last_update in both
directions, as scheduler/ownership.py and mirror_pg_periodic_tasks.py already do.

The new row inherits pg_owned and both enabled flags from the row it is split from
instead of hardcoding Beat. In a PG-adopted environment the daily and monthly tiers
had no firer at all while the hourly run still returned success.

It also moves to minute 20. Minute 0 collides with */15 — and so does the suggested
minute 30, since */15 fires at :00 :15 :30 :45 — and the per-tier locks are built so
the two runs cannot block each other.

An unrecognised tier is now rejected in post(), and the blanket except ValueError in
_run is gone, so a ValueError from inside the aggregation reaches the logged 500 path
rather than reading as a bad request body.

Tests: the JSON round-trip assertion was a stdlib tautology that never read what the
migration writes — replaced with an assertion on the updated row, the one firing the
hourly tier in production. The ALL default is pinned off inspect.signature. The RunSQL
table and column are derived from the model rather than grepped. The planner-choice
assertion is deleted: a cost model on 12,000 rows is not production evidence.

Also drops a full Organization count that ran on every tier for one log field, and
gives two test modules the Django bootstrap their siblings carry.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc

* UN-3973 [FIX] Address Athul's review: upsert-only monthly, per-schedule lock, honest success

The orphan delete is removed. The design agreed on this ticket (comments 44768/45016)
is a pure INSERT ... ON CONFLICT DO UPDATE; the delete was scope beyond it, and it
converts a recoverable undercount into unrecoverable loss — the daily rows that would
rebuild a deleted monthly row are exactly the ones that were missing. A stale total is
recoverable with backfill_metrics.

The reconciliation pass no longer shares a lock key with the 15-minute schedule. The
15-minute row is an IntervalSchedule and drifts against a fixed crontab, so on a shared
key the once-daily repair loses the race roughly one day in seven, returns
skipped=True and is never retried.

A run in which every metric for every org failed no longer reports success: True. The
result's success now reflects the error count, the completion log rises to WARNING, and
the worker-side guard reads skipped_reason and errors as well as skipped — it saw none
of these three did-nothing shapes before.

A failed monthly rollup is distinguishable from an empty one: upserted=0 collided with
the legitimate no-op and the no_active_orgs return.

source_window_days is validated and bounded. It arrives as JSON from a Beat row that is
editable in the admin: negative puts the window in the future, 0 never refreshes
yesterday, 365 restores the multi-month scan this ticket exists to remove.

Tests: a golden test seeds source rows, lets the real aggregation populate daily, and
compares the rolled-up monthly against the pre-change derivation computed independently
from get_documents_processed — AC-4 was claimed Met and had no equivalence assertion.
Fixture offsets derive from the month boundary rather than fixed day counts, which land
in the wrong month for the last days of any month.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc

* UN-3974 [FIX] Address Athul's review: lock covers what is written, both kwargs, graph guard

The lock is keyed by granularity written, not by enum member. ALL took a third key that
excluded nothing, so an ALL run and the scheduled hourly run wrote EventMetricsHourly
concurrently — reachable from the documented manual trigger and from the endpoint's own
"omit tier" contract. ALL now takes both keys and releases whatever it took if it cannot
take them all. Keys are namespaced by source window so the once-daily reconciliation
pass, which is never retried, is not starved by the 15-minute schedule.

source_window_days is accepted on all three legs. 0006 hard-depends on 0005, so the
reconciliation row is a certainty rather than a hypothetical, and this branch's
signatures rejected the kwarg it dispatches.

The tier predicates come from one membership table, so a member added without an entry
raises instead of acquiring the lock, iterating every org, writing nothing and returning
success.

The migration's bulk updates check their row counts. A filtered update matching nothing
reported success while leaving the old row on kwargs="{}" — every tier every 15 minutes
— alongside the new hourly row: strictly more load than before, silently.

tier is validated at the request boundary with a warning log, and an explicit null is
treated as omitted.

New test_migration_graph.py builds the migration graph, which is what catches 0006's
dependency on a node that is not on this branch; --no-migrations means nothing else does.
Lock behaviour is now exercised rather than its key string asserted, merge_schedules has
coverage at all, the equivalence file carries one absolute expectation and a frozen
clock, and the prefilter asserts the index is usable under enable_seqscan=off rather
than that the planner chose it on 12,000 synthetic rows.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KGZBF68CShem3pbUJM2tBc

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

…h-DB-load

…h-DB-load

…e metrics docs

Remediation pass over #2276. Fixes only — no aggregation behaviour changes.

High:
- test_aggregation_tier.py needed a live Redis but sat in the unit tier, which
  provides none: 8 errors, `test (unit)` red. Settings never override CACHES, so
  the suite inherits production django_redis. The lock cases now pin locmem, which
  has identical add/get/delete semantics; they run in CI for the first time.
- backfill_metrics computed start_date without truncating to midnight while the
  cron truncates, so the oldest day was written as a partial day's bucket. The
  monthly rollup now sums the persisted daily tier rather than recomputing from
  source, so that short value became permanent once past the reconcile window.
- 0006's rollback runbook named `migrate dashboard_metrics 0005` with no ordering.
  0005 and 0006 do not exist in the previous release, so that command errors after
  the image rolls back; and 0005's own row carries source_window_days, which the
  old signature also rejects. Corrected to 0004, reversed before the image.

Medium:
- source_window_days > 90 passed the view and raised inside the task, returning 500
  for a bad request. _int_arg now takes the task's own bound.
- test_a_blocked_run_releases_whatever_it_took never executed the rollback it
  claimed to prove: keys sort daily_monthly first, so ALL failed on its first key
  and the rollback loop ran zero times. Reordered; verified it now fails without
  the rollback.
- 0029's index guard checked validity only, so a hand-built (created_at DESC)
  index passed while Django recorded fields=["created_at"]. Lifted 0007's
  definition and current_schema() checks across, with tests.
- Releasing the lock keys was unisolated: one cache.delete raise replaced a
  completed run's return value and stranded the remaining keys.
- The "every 15 minutes" cadence was restated in five places the split made wrong.
  Removed the cadence from prose rather than restating it; it lives in 0005/0006.
- The rollup docstring and its test claimed a deleted day leaves the monthly total
  in place. It does not — the group survives with a smaller sum.
- README: the 7-day window is a lag ceiling, not only a downtime one; the hourly
  tier never self-repairs beyond 24h; added the deploy backfill at --days 62 (60
  misses a day when run on the 31st).
- docker-compose and the cloud chart both asserted the periodics never overlap and
  the Redis lock self-guards. The split made both false; comments corrected.

Also: ruff 0.3.4 (the pinned gate) reported 13 errors and 8 unformatted files, all
in tests this PR adds.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Aja2YP7hoVWqocsUd6jCkN

…ation reverse on the operation

Two siblings of fixes in 3a49d7479 that the first pass missed.

- TestTheLockIsPerSchedule (test_tasks.py) takes un-namespaced lock keys on the real
  django_redis cache and brackets each test with cache.clear(), which django-redis
  implements as FLUSHDB — it wipes every key in that database, and the Celery broker
  shares db 0 in the test env. Pinned to locmem like its sibling in
  test_aggregation_tier.py. Verified: the class fails 4/4 without the override on an
  unreachable Redis and passes 4/4 with it.

- test_it_builds_and_drops_concurrently greps the migration source, and the guard
  added in 3a49d7479 put "DROP INDEX CONCURRENTLY IF EXISTS" into two RAISE EXCEPTION
  messages — so the assertion held whatever reverse_sql was. Asserted on the rendered
  operation, as the 0007 sibling already does. Verified: mutating reverse_sql to
  RunSQL.noop now fails the test; before this it left all nine green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Aja2YP7hoVWqocsUd6jCkN

…h-DB-load

Three small fixes from a review pass; no behaviour change on an install with
active organizations.

_run_aggregation returned on no_active_orgs before reaching the monthly rollup.
That was free while monthly was computed inside the per-org loop, but UN-3973
made the rollup org-agnostic — it sums the whole daily tier and takes no org
argument — so the early return now skips it entirely. An install with no
workflow execution in the prefilter's 7-day lookback never derives monthly,
including on the documented recovery path, where backfill_metrics repairs the
daily tier and the next pass is supposed to roll monthly up from it.

test_wfe_status_created_idx.py imported a model at module scope without the
django.setup() bootstrap its 15 sibling modules carry, so collection aborted
unless another module had already initialised Django. Running that directory on
its own failed, and the full run only worked because dashboard_metrics/tests is
listed first. CI is unaffected — it sets DJANGO_SETTINGS_MODULE — so this is a
local and IDE break only.

Removed a test docstring claiming an "orphan cleanup" bounded to the rebuilt
window. _rollup_monthly_from_daily performs no deletion at all; the docstring is
a leftover from an orphan-delete that was removed under review, and the test it
sits on asserts the opposite property. The test name already states it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CFv4oHBnWPThh9wAuufQeX

…tions"

This reverts commit 37fc0fd.

…g date fleet-wide

The previous commit's incomplete-month check was wrong at both ends, as an
adversarial verification pass demonstrated against a real database.

It missed the case that matters. It counted distinct dates across every
organization while the rollup writes per (organization, month, metric), so one
tenant losing a day was invisible whenever any other tenant covered it — which is
exactly the shape _collect_org_metrics produces when one tenant's metric query
fails and the run continues. Probe: a tenant's monthly total fell 80 to 70 with
errors 0 and no log line.

And it fired when nothing was wrong: a day on which no organization was active,
a fresh install, or the 1st of a month all read as gaps, so ~25 warnings a day on
a correct daily tier — naming a repair that cannot clear them, and contradicting
README.md:168.

Replaced with the exact condition: snapshot the monthly totals, roll up, report
any that fell. No calendar heuristic, so an idle day and a fresh install are
silent by construction.

Also from the same pass: the streamed rollup is back inside one transaction (a
single bulk_create wrapped all its internal batches in one, so the split traded an
atomic rewrite for a partial one); dropped schedule kwargs are logged, since the
tolerance added for rolling deploys would otherwise hide a mistyped row and run the
reconciliation pass at the 2-day default; the zero-rows warning is raised in the
backend, which both transports reach, rather than only in the PG proxy; unknown
request-body keys are rejected instead of silently ignored; and 0006's module
docstring no longer states the schedule separation unconditionally.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017yVv7w8VuDxmDg25a1Encv

…achable guards, deterministic tests

Reconciliation row: scoped to daily_monthly. Without a tier it ran ALL, and its
hourly half was both duplicated work and the only thing writing event_metrics_hourly
concurrently with the */15 run, whose per-tier lock cannot block it. Its Beat kwargs
now come from the same spec dict as the PG row rather than a hand-written JSON
string, which is what that dict says it is for.

Lock: the value carries an owner token, and release only deletes a key this run
still owns — previously a run whose lock had expired would delete whichever run
took the key next. The age-based reclaim is now scoped to pre-token values, which
is what it was written for; a token-bearing lock is recovered by its TTL only. A
fresh pre-token lock still blocks, so a rolling deploy does not steal from an old
pod that is still working.

Tests that could not fail:
- _SplitRecorder.first() ignored its filter and update() reported a match for a row
  that was not there, so 0006's ownership inheritance and its forward guard were
  both unreachable. Both now behave as a queryset does. One test asserted the new
  row lands Beat-enabled when the row it splits is missing — an outcome the guard
  makes impossible; it now asserts the RuntimeError the code actually raises.
- Four suites derived a window from one clock reading and compared it against a run
  that took another, so they failed on a day or month boundary with no code change.
  All pinned to one reading, with a probe that runs them at both boundaries.
- wfe_status_created_idx was verified only as file text. Added the planner test its
  sibling index already had, plus the indisvalid assertion UN-3972 asks for and
  nothing checked. Scoped to the predicate rather than the joined metric query,
  since which side the planner drives from is not something a synthetic fixture pins.

Comments: the lookback floor no longer claims to rescue approved_at metrics, which
it cannot; `success` no longer claims to fail the call, which it does not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017yVv7w8VuDxmDg25a1Encv

Closes the last three review sub-items.

README gains the rollback procedure beside the backfill step: 0005/0006 write task
kwargs the previous release's signatures reject, so reverting the image alone is not
enough — migrate dashboard_metrics 0004 must run from the outgoing image first, which
a platform-driven rollback skips by construction. The release blocks automated
rollback and now says so where the ops content already lives.

The "run first!" quick-start line pointed at --days=30 rather than the deploy-critical
form; it now points at the Deploy Steps section.

backfill_metrics gains the two tests its changed window arithmetic asked for: the
oldest covered day counts its whole day (red on an untruncated boundary, verified),
and --skip-daily without --skip-monthly emits its warning.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017yVv7w8VuDxmDg25a1Encv

…hotting them

The under-count check added in bbac5c6 loaded every tenant's monthly rows into a
dict before the rollup, held it across the rollup, and read them all again after.
That compared the right thing on the wrong axis: it scales with organization x
metric x project x tag, which is the axis the streaming rollup two functions below
exists to keep off the heap, and it runs hourly — synchronously inside a request
worker on the PG transport.

Replaced with one statement evaluated in the database, run before the upsert while
the stored value is still the old one, returning only the (org, month) pairs whose
total would drop. In a healthy install that is no rows.

Behaviour is unchanged: the per-tenant regression test still passes, and two new
tests pin the bound — one query regardless of tenant count, and the comparison
happening in SQL. Reverting to the snapshots fails both while the correctness test
still passes.

Reported by Greptile.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017yVv7w8VuDxmDg25a1Encv

…h-DB-load

A 12-agent review over the loop-authored tree, with the lenses the loop had been
reading itself dispatched to fresh agents instead. Findings were verified before
fixing; several were measured down or retracted rather than taken on trust.

Guards that did not guard:
- The planner test asserted a hand-copied predicate, so removing the status filter
  from get_documents_processed left it green. It now takes the SQL from the real
  query, and that mutation fails it.
- **_ignored made inspect.signature().bind() accept anything, silently voiding all
  three "declared kwargs bind to the task signature" guards — on the first PR to put
  kwargs in schedule rows. They now check named parameters; a misspelled kwarg in
  0005 fails them again.
- The release ownership check, the lowered_months propagation and the diagnostic's
  unavailable marker each had no test that could fail. All three now do.
- The backfill truncation test read the real clock and passed with the bug present
  before 01:30 UTC. Frozen.
- The maximum bound on source_window_days had no test.

Signals that did not signal:
- A failed under-count check set lowered = [], indistinguishable from "checked,
  nothing lowered", and the backend's log is in a different process from the
  consumer watching the schedule. The payload now carries lowered_check.
- monthly.failed was written and read by nothing, so a fleet-wide rollup failure
  read as one org's metric error. The worker now reports both.
- "tier wrote no rows" fired on any quiet install, because the prefilter shortlists
  7 days while the hourly tier queries 24h. Scoped to daily_monthly.

Lock:
- Removed the age-based reclaim. It could not tell a live holder from a dead one,
  its read-judge-replace was not atomic, and it was unreachable — the keys are new
  in this release, so no pre-token value can appear under one. Its two tests went
  with it, replaced by the property that is reachable.
- The acquire loop now releases what it took if it raises partway.

Migrations:
- 0005 inherits scheduler ownership instead of hardcoding pg_owned=False. The
  rescue previously relied on is pgScheduleMirror, which ships disabled.
- 0006 creates a missing row instead of raising, which failed migrate for every app
  in the project on a state that cannot cause the harm the guard described.

Docs: the rollback section carries the post-hoc recovery for the case it predicts;
the **_ignored rationale was inverted at three sites; five sites still said the
reconcile pass covers all tiers; the created_at index census was wrong.

Measured down rather than fixed as reported: the diagnostic is 0.3x the rollup at 37
orgs, not 240x (that came from a 300-org fixture) — only the redundant isnull
conjunct was real, and is gone. The deadlock needs a >20 min run (measured 0.5s) and
a concurrency change. backfill_metrics' silent success is a pre-existing swallow.

Verified on a disposable Postgres with migrations applied: both indexes built and
indisvalid, all five schedule rows correct on both transports, reverse to 0004 and
re-apply both clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017yVv7w8VuDxmDg25a1Encv

mypy is commented out in .pre-commit-config.yaml, so a positional swap has no
enforcement at all. _aggregate_single_metric took 11 positional parameters
including three adjacent datetimes (hourly_start, daily_start, end_date) and two
identically-typed dicts (hourly_agg, daily_agg); transposing either pair produces
wrong numbers with no error and no failing test.

Each of the four helpers now takes one positional subject and nothing else. The
enforcement is structural — Python raises TypeError — rather than a convention.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017yVv7w8VuDxmDg25a1Encv

Each gap was reproduced with the mutation the reviewer named, and each new test
was confirmed to die on that mutation while the pre-existing tests stay green.

T1 — the task's lock round trip. His claim is overstated at the helper level:
`_release_aggregation_locks` IS covered, and gutting it kills two existing tests.
What is uncovered is one level up — the task's own `finally`. The only test that
drives `aggregate_metrics_from_sources` through the lock patches both the acquire
helper and `cache`, so replacing the `finally` body with `pass` left the suite
fully green. TestTheTaskAcquiresAndReleasesForReal drives the task against a real
locmem cache: keys held during the run, released after, a re-entrant second run
locked out, and keys released when the run raises. His TTL point is confirmed —
AGGREGATION_LOCK_TIMEOUT and the hourly period are both 900s, so a broken release
skips a run while still returning {"success": True, "skipped": True}.

T2 — the guard was only ever seeded inside one month, so the per-month
correlation was inert. Deleting `.filter(bucket=OuterRef("month"))` leaves the
five existing guard tests green. The rollup window starts at the first of the
PREVIOUS month, so uncorrelated it compares one month's stored total against two
months of daily rows: the sum is always larger and nothing is ever flagged.
Worth noting the mutant also FALSELY flags a healthy month, which makes the
rollup skip a correct month's upsert and freeze it.

T3 — neither index migration's `DO $$` block was ever parsed by a server. Under
--no-migrations the migrations never run, and the existing test asserts on the
SQL as Python substrings, so breaking BEGIN to BEGINN in both files leaves the
suite green while `migrate` would fail on deploy. The new test executes both
guards against Postgres, and exercises the two cases they exist for: an index of
the same name built descending (the slip IF NOT EXISTS cannot catch) and a
missing one.

T4 — 0005's `_inherited_ownership` was covered only on its `is None` fallbacks,
because the existing tests run against a clean DB where the aggregation row does
not exist; hardcoding {True, True, False} passes all four. The new test seeds a
PG-adopted fleet and asserts the reconciliation row follows it. 0006's identical
helper was already covered, which is what made the asymmetry visible.

Not a finding: the "order-dependent suite" reported alongside these. pytest-
randomly is not a dependency (absent from pyproject.toml and uv.lock), so there
is no shuffling; the varying results came from concurrent runs sharing one test
database. Three consecutive runs are identical at 185 passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017yVv7w8VuDxmDg25a1Encv

Two of these are defects in code the previous commits added, both High.

The coverage check counted today in the numerator while measuring against
yesterday in the denominator, so today's row cancelled exactly one missing
earlier day. Since the daily/monthly schedule runs at :20, today normally has
rows — meaning a SINGLE missed aggregation day, the case the check exists for,
was silent, and it only fired at two or more. It also flapped: the same state
reported a gap at 00:20 and nothing at 09:20. Both sides now count whole days
only. My own tests passed because the fixture stopped at day 9 while "today" was
day 10, so the pair never exercised the production shape — a gap WITH today's
row present. That case is now a test.

_run_diagnostic's broad `except Exception` swallowed SoftTimeLimitExceeded — the
exact hazard the three re-raises in the previous commit exist to close, and this
was a fourth site introduced alongside them. Swallowed, the check returns [] and
the rollup runs its fleet-wide upsert with the guard OFF, overwriting the totals
the guard was preserving, with under a minute left before the hard limit.
Re-raised here and at the rollup's own catch.

Two docstring claims were false and are corrected rather than defended: "a fresh
install is silent" holds only for an install starting on the 1st, and the check
cannot see a day lost by a single tenant or metric — one row from anyone marks
the date covered. The warning no longer prescribes a backfill unconditionally,
because a date on which nothing ran anywhere reads identically and needs none.

Nothing crossed the reporting seam for the new key: returning [] from the check,
or typoing `incomplete_daily_coverage`, left the suite green while the control
typo on `needs_daily_repair` failed. Now covered end to end.

From the simplify pass: `_report` extracts the two identical stats-plus-warning
blocks; `_collect_metrics` returns its failure count instead of writing an
instance attribute, matching `_collect_org_metrics`; the worker binds
`result["monthly"]` once instead of five times and drops a docstring that
enumerated the arms and had already gone stale against them; `lowered_check`'s
log said "lowered" where nothing was lowered; over-long comments cut to the rule
in CLAUDE.md.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017yVv7w8VuDxmDg25a1Encv

The adversarial pass noted B5's message change was invisible to the suite:
reverting `"... and N more of TOTAL"` to the old `"... and more (showing 20)"`
left everything green. Same for the worker's new coverage arm. Both are now
mutation-checked — each fails with its production line reverted and passes with
it restored.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017yVv7w8VuDxmDg25a1Encv

The simplify agents read up to ed582c2 and the adversarial verifier read
244853e, so the last two commits had no independent pass. Reading them back:

_run_diagnostic's docstring said it records unavailability "rather than raising",
which stopped being true the moment SoftTimeLimitExceeded was made to propagate
through it. Corrected, and cut to the size of the body.

The comment on the backfill's failure counter referenced "the PR" — meaningless
once merged — and carried "used to" history. Rewritten to state the mechanism.

Left alone deliberately: the same "used to" phrasing in two TEST docstrings. For
a test, the behaviour it guards against is the reason the test exists, and the
surrounding suite already documents regressions that way.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017yVv7w8VuDxmDg25a1Encv

…urface

A verifier given the files and one question — "can each actor do what it is told?"
— with the design rationale deliberately withheld. It found five things the
context-carrying passes did not, all on the half nobody had been reading: the
operator's and on-call's.

Verified against a disposable Postgres, not asserted:

The README's contingency recovery is a one-way door. Its SQL changes rows, not
`django_migrations`, so a later roll-forward reports "No migrations to apply" and
restores nothing — measured: 5 rows, recovery SQL, 3 rows, roll forward, still 3.
The install then runs the single */15 row with kwargs '{}', which defaults `tier`
to ALL: every tier 96 times a day, the load this release removes, and no
reconciliation pass, with `tier=all` in a log line as the only trace. Documented,
with the `migrate dashboard_metrics 0004 --fake` step that undoes it — also
measured: 3 rows, --fake, forward, 5 rows with kwargs intact.

The README told the operator to run the backfill "before the first aggregation",
which `tasks.py` says outright cannot be produced — 0006's row goes live at the
end of `migrate`. It now says what actually happens and what it costs.

The Quick Commands block told the operator to run the deploy form BEFORE
deploying. `--skip-hourly` only skips the HOUR queries as of this release; from
the outgoing image it issues them anyway, which is the scan this change exists to
remove.

Two worker arms: the "wrote no rows" warning had no tier guard, where the backend
guards the identical condition with `_writes_daily_monthly` and says why — on the
*/15 row it warns 96 times a day about a healthy weekend. And the
incomplete-coverage line dropped the "needs no action" clause from the backend's
message it claims parity with, telling on-call to run a 62-day backfill that
cannot clear the condition.

Deferred: adding a third pre-rollup check reaches `tasks.py` cleanly and reaches
on-call not at all — every result key is hand-enumerated in the worker, in a
different image, with no shared constant. The fix is a shared key table, which is
a cross-cutting change and does not belong in this PR.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017yVv7w8VuDxmDg25a1Encv

…--help

A second cross-document sweep, this time keyed on the FLAG NAMES rather than on
the behaviour words, found a block presenting itself as `--help` output whose
three `--skip-*` lines still describe the pre-guard behaviour — the same class as
the six sites already fixed, in a place neither the review nor the first sweep
reached because it names no behaviour. It was also missing `--active-only`.

The lines now match the real help, and the block says outright that `--help` is
authoritative, since the caveats on those three flags do not fit one line and
duplicating them is what let this drift in the first place.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017yVv7w8VuDxmDg25a1Encv

The previous commit corrected the README's reproduction of this flag to "skips
the HOUR source queries, not just their upsert" and labelled `--help` the
authoritative text. The help itself still said "Skip hourly aggregation (only do
daily/monthly)" — so the fix pointed readers at a less accurate statement than
the one it replaced.

The distinction is the whole reason the flag matters to the deploy step: the
window is measured in weeks, and issuing the HOUR queries anyway reinstates the
scan this release removes. The docstring on `_collect_metrics` and the test
`TestSkipHourlySkipsTheQueries` both already say so; only the help did not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017yVv7w8VuDxmDg25a1Encv

Two High, and the first one is a correction to my own verification.

The README told an operator who had hand-run the recovery SQL to run
`migrate dashboard_metrics 0004 --fake` before rolling forward. At that moment
the operator is on the OUTGOING image — the paragraph above says so — where
0005 and 0006 are not on disk, so 0004 has no child to unapply and the command
prints "No migrations to apply" while un-recording nothing. I had verified this
step and reported it proven; I verified it on the incoming image, where the files
exist, which is the one image the operator is not standing on. Measured both ways:
files absent -> empty plan; files present -> both unapply. The text now says to
deploy the new image first and run two commands from it, and says what happens if
you run it from the old one.

test_an_incomplete_daily_tier_reaches_the_result_dict skipped only when no whole
day had elapsed, so on the 2nd of every month it ran with exactly one elapsed day,
its single seeded row covered it, and the assertion raised KeyError — the whole
backend suite red for every PR in the repo, one day a month, reading as a rollup
regression. Guard widened; replayed across the 1st, 2nd and 3rd to confirm.

Five lines the diff added were constrained by nothing — each mutated cleanly with
both suites green:
  coverage_check's payload key (its sibling needs_daily_repair was pinned, this
    one was not — the same seam, half-covered)
  the worker's two logger.warning -> logger.error upgrades, invisible because
    every worker test asserts on caplog.text, which cannot see severity
  the LLM arm's failures += 1
  0006's "interval": None (verified on a real database, but nothing in CI held it)
All five now fail under the mutation that exposed them.

Also ran the pinned ruff over the new test file, which it had never seen.

Not fixed, deliberately: the coverage warning repeats ~25x/day for up to 62 days
after a single fleet-idle day. The suggested fix — only report contiguous gaps —
would reintroduce the exact defect this check was added for, since a one-day
outage is indistinguishable from an idle day by contiguity. Needs a product call
about warn frequency, not a patch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017yVv7w8VuDxmDg25a1Encv